Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Add support for rendering media captions #43

Merged
merged 9 commits into from
Oct 4, 2024
Merged

Add support for rendering media captions #43

merged 9 commits into from
Oct 4, 2024

Conversation

@tulir
Copy link
Contributor

@tulir tulir commented Sep 13, 2024 •
edited
Loading

Depends on matrix-org/matrix-js-sdk#4403

Preview:

image

When hovering over image

image

@tulir tulir requested a review from a team as a code owner September 13, 2024 11:52
@tulir tulir requested review from dbkr and robintown September 13, 2024 11:52
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Sep 13, 2024
dbkr
dbkr reviewed Oct 2, 2024
View reviewed changes
Copy link
Member

@dbkr dbkr left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good - this will probably need a look from design though. Could you add a screenshot to make it easier?

@tulir
Copy link
Contributor Author

tulir commented Oct 2, 2024

Added screenshot

@dbkr dbkr requested a review from a team October 2, 2024 14:04
@dbkr
Copy link
Member

dbkr commented Oct 2, 2024

and what a screenshot it is. Requested design review, with promise of cats.

amshakal
amshakal approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@amshakal amshakal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
th-4091376590

@t3chguy t3chguy requested a review from dbkr October 4, 2024 11:29
@dbkr dbkr added this pull request to the merge queue Oct 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 4, 2024
@tulir
Copy link
Contributor Author

tulir commented Oct 4, 2024

Not sure why that test only failed in the merge queue 🤔 I guess I'll add another mock there

@t3chguy t3chguy enabled auto-merge October 4, 2024 13:24
@t3chguy t3chguy added this pull request to the merge queue Oct 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 4, 2024
@t3chguy t3chguy added this pull request to the merge queue Oct 4, 2024
Merged via the queue into element-hq:develop with commit 5fbc5af Oct 4, 2024
27 checks passed
@tulir tulir deleted the captions branch October 4, 2024 14:26
@frebib
Copy link

frebib commented Oct 5, 2024

This works great with modern layout, but is quite broken with bubble layout. Some captions are not visible with image bubbles 🤔

@tulir
Copy link
Contributor Author

tulir commented Oct 5, 2024

🙀

@mwnu mwnu mentioned this pull request Oct 29, 2024
Open
This was referenced Oct 30, 2024
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@robintown robintown Awaiting requested review from robintown robintown is a code owner automatically assigned from element-hq/element-web-reviewers

2 more reviewers

@dbkr dbkr dbkr approved these changes

@amshakal amshakal amshakal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

T-Enhancement Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tulir @dbkr @frebib @amshakal @t3chguy